fix(imap): detect iMIP when method= is missing or mime is application/ics#12790
fix(imap): detect iMIP when method= is missing or mime is application/ics#12790lubo92 wants to merge 1 commit into
Conversation
|
Hi @lubo92 Thanks for the PR! I think the method discovery can actually be removed, the back end code no longer has split handling for calendar invitations, like it did before, There for the external (pre ics parsing) method discovery in the mail app, no longer really has a use. The method discovery now happens after the calendar parses the event. The only thing I would leave is the ics file discovery based on type text/calendar and application/ics @kesselb what do you think? |
|
Thanks for your PR! We've discussed this a while ago: #11009 I reached out to Proton back then, and they confirmed the bug and told me they would add it to their roadmap. I would kindly ask you to reach out and flag it again, as that hopefully raises the chances of having this issue resolved properly at some point. My request ticket number was 3648275. Why did you also add application/ics? The RFC is quite strict on this, it should be text/calendar. Is there a provider generating those with application/ics? I think we still need the code path, as it adds the invitation to the scheduling array which is later used to render the iMIP component. I'm not aware that we want to get rid of it. mail/lib/IMAP/ImapMessageFetcher.php Lines 306 to 338 in 95442ad |
A lot of providers add this. |
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
…/ics Two upstream-detection gaps let calendar invitations slip through: 1. Proton Mail Bridge strips the `method=` parameter from the text/calendar Content-Type header while re-assembling a message after E2E-decryption. `MessageMapper::getBodyStructureData()` requires that parameter to flag the message as iMIP, and `ImapMessageFetcher::getPart()` requires it to populate the scheduling list — so every Bridge-delivered invitation is silently dropped. 2. Google's calendar invitations occasionally ship the same event twice: once as text/calendar and once as application/ics. Upstream ignored the application/ics part entirely, which means a mail client fed the "wrong" part (some MUAs prefer the second attachment) saw nothing. This commit widens both detection sites to also accept application/ics, and in the fetcher falls back to the METHOD: line inside the ICS body when the Content-Type parameter is missing. The body is only loaded in the fallback path, so the common case (method= present) keeps the same I/O pattern it had before. Added two parameterised test fixtures: - request_proton_bridge.txt (text/calendar, no method= param) - request_application_ics.txt (application/ics) Both are expected to now be flagged as iMIP. AI-assisted: Claude Code (Claude Opus 4.7) Signed-off-by: Wolfgang Lubowski <w.lub92@gmail.com>
lubo92
force-pushed
the
fix/imip-detection-without-method-param
branch
from
a4351f8 to
7c15528
Compare
|
Thanks @SebastianKrupinski and @kesselb for the careful read. Quick update plus answers below. Update on the PR itself
On scope (re @SebastianKrupinski) Agreed that the calendar backend now does its own METHOD discovery after ICS parsing in So I kept the codepath and only widened the discovery in two minimally invasive ways:
Happy to revisit if you'd rather drop the REQUEST/REPLY/CANCEL filter as well and let the backend decide — but it felt safer to keep the filter as-is for this fix. On You're right that RFC 5546 is strict on On Proton (re @kesselb) Will reach out to Proton again referencing ticket 3648275 — I agree the proper fix belongs upstream in Bridge. Treating this PR as the defensive fallback so Bridge users aren't silently losing invitations until that lands. PTAL when you have a minute. |
| $contents = null; | ||
| if ($method === null) { | ||
| $contents = $this->loadBodyData($p, $partNo, $isFetched); | ||
| if (preg_match('/^METHOD:\s*(\S+)/mi', $contents, $m)) { |
There was a problem hiding this comment.
| if (preg_match('/^METHOD:\s*(\S+)/mi', $contents, $m)) { | |
| if (preg_match('/^METHOD:([A-Z]+)/mi', $contents, $m) === 1) { |
|
Thanks for the update. With the current information (provided by you and the original report), I am okay with looking into the attachment to check if it looks like an event invitation. For a Google invitation damaged by Proton, the actual fix is obtaining the method from the attachment. Nothing else needed. Looking at application/ics seems nice, but in this case it is a nice-to-have on the side. I would prefer to not do that. If someone reaches out with a provider that only includes application/ics, we can reevaluate (in case they are not fixing their implementation). That matches the current state in Thunderbird.
It does not. I created that sample based on an original Google invitation, but dropped all unnecessary parts including the base64 encoded application/ics part. |
3 participants
Summary
Two upstream-detection gaps let calendar invitations slip through unprocessed:
1. Proton Mail Bridge strips the
method=parameter. During E2E re-assembly Bridge loses the Content-Type parameter on the text/calendar part:instead of
The ICS body still contains
METHOD:REQUEST, but `MessageMapper::getBodyStructureData()` and `ImapMessageFetcher::getPart()` both key off the Content-Type parameter and ignore the body. Result: every single Proton-Bridge user silently loses inbound invitations from Gmail, M365, Fastmail, …2.
application/icsis ignored. Google's invitations ship two calendar parts — onetext/calendar, oneapplication/ics. If the calling code hands the fetcher theapplication/icspart (some MUAs/filters prefer the alphabetically-second attachment), it is treated as a regular attachment and no scheduling entry is built.Changes
method=precondition (the method resolution happens further down).method=parameter, parses `METHOD:` from the ICS body instead. Body is only loaded in the fallback path, so the common case keeps its I/O pattern unchanged.Tests
Two new fixtures + data-provider entries in `MessageMapperTest::testGetBodyStructureIsImipMessage()`:
method=parameter, METHOD:REQUEST in bodyBoth must now evaluate to `isImipMessage() === true`.
Real-world impact
Empirically verified on a production Proton-Business + Nextcloud 32 + Mail-App 5.7.12 setup: before this change, Gmail and M365 invitations delivered via Bridge never reached CalDAV. After applying the same fix in-place, the iMIP pipeline works end-to-end for requests, replies, and cancels. Repro MIME samples available on request.
Checklist
method=parameterCo-authored-with Claude Opus 4.7.